Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defer updating the animations Tree in SpriteFramesEditor to avoid crashes #81643

Conversation

Rindbee
Copy link
Contributor

@Rindbee Rindbee commented Sep 14, 2023

Previously, clicking the LMB while renaming an animation could cause SpriteFramesEditor::_update_library(false) to be called during Tree::propagate_mouse_event(). This may cause a crash.

godot/scene/gui/tree.cpp

Lines 3783 to 3784 in 3ed4497

blocked++;
propagate_mouse_event(pos + theme_cache.offset, 0, 0, x_limit + theme_cache.offset.width, mb->is_double_click(), root, mb->get_button_index(), mb);

godot/scene/gui/tree.cpp

Lines 4290 to 4291 in 3ed4497

TreeItem *Tree::create_item(TreeItem *p_parent, int p_index) {
ERR_FAIL_COND_V(blocked > 0, nullptr);

TreeItem *it = animations->create_item(anim_root);
it->set_metadata(0, name);

We can defer updates to the editor interface to avoid calling Tree::create_item() at the wrong time.

Fix #79149.

@Rindbee Rindbee force-pushed the defer-rebuild-the-animations-tree-in-SpriteFramesEditor branch 3 times, most recently from 0b81926 to a5d1334 Compare October 17, 2023 02:17
@Rindbee
Copy link
Contributor Author

Rindbee commented Oct 17, 2023

A mechanism similar to update_tree() in EditorInspector is used.

@Rindbee Rindbee force-pushed the defer-rebuild-the-animations-tree-in-SpriteFramesEditor branch 2 times, most recently from 68459d7 to a1449f7 Compare October 17, 2023 12:16
@Rindbee
Copy link
Contributor Author

Rindbee commented Oct 17, 2023

Fix the edited animation not updating when undoing.

Before:

0.mp4

After:

1.mp4

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't really reproduce the crash to test the fix, but the changes look fine.

…shes

Previously, clicking the LMB while renaming an animation could cause
`SpriteFramesEditor::_update_library(false)` to be called during
`Tree::propagate_mouse_event()`. This may cause a crash.

We can defer updates to the editor interface to avoid calling
`Tree::create_item()` at the wrong time.

Enables `SpriteFramesEditor::_select_animation()` to be able to undo/redo
@Rindbee Rindbee force-pushed the defer-rebuild-the-animations-tree-in-SpriteFramesEditor branch from a1449f7 to 2642c68 Compare October 17, 2023 22:46
@akien-mga akien-mga merged commit 5fd3354 into godotengine:master Oct 18, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@Rindbee Rindbee deleted the defer-rebuild-the-animations-tree-in-SpriteFramesEditor branch October 18, 2023 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch animation while renaming other animation in SpriteFrames crash
3 participants